Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace isparta with babel-plugin-istanbul #402

Merged
merged 1 commit into from
Dec 30, 2016

Conversation

posva
Copy link
Contributor

@posva posva commented Dec 8, 2016

Some attention points:

  • Used arrays on the include webpack.base field because it's more flexible
  • Changed the include path to the src directory
  • Added cross-env when launching karma. NODE_ENV=test is necessary for coverage

@@ -7,7 +7,7 @@
"scripts": {
"dev": "node build/dev-server.js",
"build": "node build/build.js"{{#unit}},
"unit": "karma start test/unit/karma.conf.js --single-run"{{/unit}}{{#e2e}},
"unit": "cross-env NODE_ENV=test karma start test/unit/karma.conf.js --single-run"{{/unit}}{{#e2e}},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably best to use BABEL_ENV

Babel will use either of these, but will probably be more clear of the use and cause less side affects

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@blake-newman Thanks! I didn't know it also worked that way

- Used arrays on the include webpack.base field because it's more
flexible
- Changed the include path to the src directory
- Added cross-env when launching karma. This is necessary for coverage
Copy link
Contributor

@blake-newman blake-newman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably use babel-istanbul-loader

There are some issues with the syntax highlighting with plugin eg *.vue files. The plugin also does not cover vue files correctly, the values are wrong.

Works really nicely for me, looking at the source the loader and plugin do not do much different. As they both use babel-istanbul. Just how the files get parsed to babel-istanbul.

Developer experience is key with Vue, so i’d rather go with the most appropriate solution, to enable as much awesomeness as possible.

babel-plugin-istanbul:

screen shot 2016-12-09 at 12 35 59

babel-istanbul-loader:

screen shot 2016-12-09 at 12 42 00

@LinusBorg
Copy link
Contributor

@posva @blake-newman What's the status on this?

@posva
Copy link
Contributor Author

posva commented Dec 24, 2016

@LinusBorg There's another thing that we should use. I'm looking into it 😆

edit: https://github.com/deepsweet/istanbul-instrumenter-loader
edit2: i'm looking into it

@posva
Copy link
Contributor Author

posva commented Dec 24, 2016

@blake-newman I tested the other lib but it doesn't work well. The babel plugin is the recommended way: https://github.com/istanbuljs/istanbul-lib-instrument
The last thing we can try is fork the project and update the dependencies but we're probably not going to update it. IMHO I prefer having an official plugin that is more likely being maintained that syntax colours in the coverage report. WDYT?

@blake-newman
Copy link
Contributor

Yeah I'd agree the best library for maintainability.

However the development experience with Vue files is a little raw. Syntax and styling is off.

Two solutions:
Fork babel-istanbul-loader and self maintain or
see how much effort is required to improve the plugin to support html based files.

Maybe a quick investigation on the later would be good, as it seems alot of tools seem to use the official version.

IMO I think creating a babel-vue-script-parser plugin would be nice to extract html. This would be an agnostic solution to parse Vue file script content.

Tools like Ava, Jest, Istanbul ect would really benefit from a plugin like this.

I would certainly suggest this approach, as Ava is an amazing testing tool. That has alot of React like testing Utils that we could leverage. Like snapshot testing.

@blake-newman
Copy link
Contributor

/cc @posva

@blake-newman
Copy link
Contributor

Note this is similar to a piece of.work I did to get stylelint to cover style tags. I could easily create a plugin to mimic this for script tags with babel.

@LinusBorg
Copy link
Contributor

@blake-newman @posva I think Blake's approach would be nice - but would take some time, right?

Could we add the babel-plugin for instanbul for now with this PR, and work on the 'babel-vue-script-parser' idea after that?

@blake-newman
Copy link
Contributor

Yeah certainly

@posva
Copy link
Contributor Author

posva commented Dec 30, 2016

@LinusBorg @blake-newman LGTM!

@LinusBorg LinusBorg merged commit ff9c817 into vuejs-templates:master Dec 30, 2016
@aasheer
Copy link

aasheer commented May 22, 2017

I know this is closed but I'm pulling my hair out trying to find a solution to what I hope is a related issue.

We are building a Vue application using the vue-cli Webpack template. The only change we made was to opt for using Ava for unit testing along with istanbul/nyc as the reporter/coverage tool. Our tests pass, the .vue files show up in the nyc report result but the line numbers are off. By that I mean that nyc is pointing to lines 700, 706, etc. as uncovered but the .vue component being tested only has 400 lines in it. I've been hearing about making sure I have sourceMap: true in my nyc package.json config but I tried setting that and a few other things. Nothing seems to work.

So my overall question is:

Why is nyc pointing to line numbers that don't exist when I'm testing a .vue component file with ES6 code (and not transpiled files)? If nyc is somehow looking at some transpiled version of my .vue file, how do I map it back (with the correct line numbers) to my .vue component file?

@blake-newman
Copy link
Contributor

blake-newman commented May 23, 2017

@aasheer Please see the new recipe for ava.js and vue, this should help. https://github.com/avajs/ava/blob/master/docs/recipes/vue.md

@aasheer
Copy link

aasheer commented May 23, 2017

@blake-newman, thanks for the response. yeah a couple of people suggested your recipe. I have the following in my package.json:

    "nyc": {
    "exclude": [
      "build",
      "config",
      "static",
      "tests"
    ],
    "extension": [
      ".js",
      ".vue"
    ]
  },
  "ava": {
    "babel": "inherit",
    "require": [
      "./tests/unit/helpers/setup.js",
      "ignore-styles",
      "babel-core/register"
    ]
  },
  "babel": {
    "presets": ["es2015"],
    "plugins": ["transform-runtime"],
    "ignore": "**/*.test.js",
    "env": {
    	"test": {
    		"sourceMaps": "inline",
            "plugins": ["transform-runtime"]
    	}
    }
  },

I have the following in my setup.js file:

// Setup browser environment
require('browser-env')();
const hooks = require('require-extension-hooks');
const Vue = require('vue');

// Setup Vue.js to remove production tip
Vue.config.productionTip = false;

// Setup vue files to be processed by `require-extension-hooks-vue`
hooks('vue').plugin('vue').push();
// Setup vue and js files to be processed by `require-extension-hooks-babel`
hooks(['vue', 'js']).plugin('babel').push();`

when I run my unit tests "npm run unit-test" I get the following error:

`/local/content/nci-strap-ui/tests/unit/specs/DT4.test.js:1
(function (exports, require, module, __filename, __dirname) { import _typeof from 'babel-runtime/helpers/typeof';
                                                              ^^^^^^
SyntaxError: Unexpected token import
```


I'm at my wits end. No clue what I'm doing wrong. I've tried a variety of combinations but to no avail. Any help would be appreciated. 

Thank you.

@LinusBorg
Copy link
Contributor

(Sidenote: use 3 backticks oto format blocks of code. I edited your comment accordingly.)

@blake-newman
Copy link
Contributor

Looks to me that the issue resides in the order of the helpers for Ava perhaps. Maybe change the setup helper to run last.

I would need a reproduction if possible to help further.

"ava": {
    "babel": "inherit",
    "require": [
      "ignore-styles",
      "babel-core/register",
      "./tests/unit/helpers/setup.js"
    ]
},

@aasheer
Copy link

aasheer commented May 24, 2017

@blake-newman , I really appreciate the help. Still no luck with your suggestion above, but if it helps at all, here is the default Vue-Cli webpack boilerplate .babelrc file code:

{
  "presets": [
    ["env", { "modules": false }],
    "stage-2"
  ],
  "plugins": ["transform-runtime"],
  "comments": false,
  "env": {
    "test": {
      "presets": ["env", "stage-2"],
      "plugins": [ "istanbul" ]
    }
  }
}

So that's the last piece. I'm assuming "babel":"inherit" in the ava package.json config inherits from .babelrc file.

@aasheer
Copy link

aasheer commented May 24, 2017

@blake-newman, I think I finally got it to work! Although I'm not sure why. The only thing I can definitely put my finger on is that if I put "babel":"inherit" in my ava package.json configuration, I get the error. What I ended up doing is leaving my .babelrc file as is, and defining a babel config in my package.json file. The working config looks like this:

  "ava": {
    "require": [
      "ignore-styles",
      "babel-core/register",
      "./tests/unit/helpers/setup.js"
    ]
  },
  "babel": {
  	"presets": [
        ["env", { "modules": false }],
        "stage-2"
    ],
  	"plugins": ["transform-runtime"],
    "comments": false,
  	"ignore": "**/*.test.js",
  	"env": {
  		"development": {
            "presets": ["env", "stage-2"],
  			"sourceMaps": "inline"
  		}
  	}
  },

I have to look into all the stuff for babel config but I got some of the config above from the official Ava code-coverage recipe. That being said, I still have to define it in package.json and not in .babelrc

Now however, nyc returns actual line numbers that map to my .vue file. If you feel there is anything to add, that I might not be considering, please let me know. Again, thanks for your efforts and the helpful information.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants